Skip to content

reactions: Add sheet to view who reacted to a message. #1243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

reactions: Add sheet to view who reacted to a message. #1243

wants to merge 2 commits into from

Conversation

@chimnayajith
Copy link
Contributor

@chimnayajith chimnayajith commented Dec 31, 2024 •
edited
Loading

Description

This pull request introduces a Reaction List sheet which can be accessed from either the context menu or by long pressing the reaction chips. Currently the app shows who reacted when there are up to 3 of them. But once there are more, we just show counts, and the user may want to see specifically who left the reactions.

Related Issues

Closes #740

Screenshots

Dark Mode

Light Mode

@chrisbobbe

This comment was marked as outdated.

Sign in to view
@chrisbobbe chrisbobbe mentioned this pull request Jan 2, 2025
Closed
@chrisbobbe
Copy link
Collaborator

Thanks! This PR needs tests.

Also, please reorder the two commits, so the commit that adds the icon comes first. That way the icon will be available for the commit that adds the feature.

@chimnayajith chimnayajith force-pushed the reaction-list branch 3 times, most recently from 3667ec4 to 6bfe471 Compare January 16, 2025 05:33
@chimnayajith
Copy link
Contributor Author

chimnayajith commented Jan 16, 2025 •
edited
Loading

@chrisbobbe I've reordered the commits and written the tests for the same. Please take a look. Thank you.

chrisbobbe
chrisbobbe reviewed Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Comments below from a partial review.

Also, a question for you 🙂: what happens if you're looking at the tab for a particular reaction, and then the last vote for that reaction is removed?

@chimnayajith chimnayajith force-pushed the reaction-list branch 2 times, most recently from 176c00a to dde36b5 Compare January 24, 2025 16:41
@chimnayajith
Copy link
Contributor Author

I’ve addressed all the requested changes and marked them as resolved.

As per the conversation here, I’ve implemented the live UI using the MessgeListView getter .

As for the question:

What happens if you're viewing the tab for a particular reaction, and then the last vote for that reaction is removed?

Currently, when the last vote for a reaction is removed, the corresponding tab disappears, and the view switches to the previous tab. If there are no remaining reactions, a message saying 'No reactions yet' is shown. Does this behavior seem appropriate?

@chrisbobbe
Copy link
Collaborator

Currently, when the last vote for a reaction is removed, the corresponding tab disappears, and the view switches to the previous tab. If there are no remaining reactions, a message saying 'No reactions yet' is shown. Does this behavior seem appropriate?

Yes, that sounds reasonable. Thanks for explaining!

@chimnayajith chimnayajith force-pushed the reaction-list branch 2 times, most recently from 0e53322 to ec9196d Compare February 1, 2025 09:57
@chimnayajith chimnayajith changed the title reactions: Add sheet to view who reracted to a message. reactions: Add sheet to view who reacted to a message. Feb 2, 2025
@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Feb 4, 2025
@gnprice
Copy link
Member

gnprice commented Feb 6, 2025

Thanks @chimnayajith for picking this up!

This is a post-launch issue, and it looks like there's still substantial work to do in order to (a) get this matching all the details of the design in Figma, and (b) get the structure of the code up to our standards. So as part of focusing on launch issues until the launch is ready, let's put this PR on hold for now.

@gnprice gnprice added this to the M6: Post-launch milestone Feb 6, 2025
@gnprice gnprice marked this pull request as draft February 6, 2025 04:19
@gnprice gnprice mentioned this pull request Feb 6, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrisbobbe chrisbobbe Awaiting requested review from chrisbobbe

Assignees

No one assigned

Labels

maintainer review PR ready for review by Zulip maintainers

Projects

None yet

Milestone

MXA: Later

Development

Successfully merging this pull request may close these issues.

See who left an emoji reaction

3 participants

@chimnayajith @chrisbobbe @gnprice